Skip to content

Use new cached project count column (part 2 of 2)#805

Merged
zetter-rpf merged 1 commit into
mainfrom
cache-submitted-count-part-2
May 18, 2026
Merged

Use new cached project count column (part 2 of 2)#805
zetter-rpf merged 1 commit into
mainfrom
cache-submitted-count-part-2

Conversation

@zetter-rpf
Copy link
Copy Markdown
Contributor

@zetter-rpf zetter-rpf commented May 1, 2026

MUST BE DEPLOYED AFTER #804

Status

Related to: https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1313
Follows on from: #804

What's changed?

In part 1 we started caching the value of submitted projects. Now we can use that value to simplify the code and reduce N+1 queries.

@cla-bot cla-bot Bot added the cla-signed label May 1, 2026
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 1, 2026

Test coverage

91.08% line coverage reported by SimpleCov.
Run: https://github.com/RaspberryPiFoundation/editor-api/actions/runs/26024856474

@zetter-rpf zetter-rpf force-pushed the cache-submitted-count branch from 9def7db to 6ff71db Compare May 1, 2026 15:28
@zetter-rpf zetter-rpf force-pushed the cache-submitted-count-part-2 branch from ed1ecb5 to a503b60 Compare May 1, 2026 15:41
@zetter-rpf zetter-rpf marked this pull request as ready for review May 1, 2026 15:41
@zetter-rpf zetter-rpf temporarily deployed to editor-api-p-cache-subm-lcmxl0 May 1, 2026 15:41 Inactive
@adrian-rpf adrian-rpf requested a review from mwtrew May 6, 2026 09:04
Base automatically changed from cache-submitted-count to main May 18, 2026 07:34
In [1] we started caching the value of submitted projects. Now we can use that value to simplify the code and reduce N+1 queries.

[1] #804

Note that I've had to split the loading of remixes for teachers and students as teachers no longer need remixes (which causes bullet to complain)

Co-authored-by: Copilot <copilot@github.com>
Copilot AI review requested due to automatic review settings May 18, 2026 09:23
@zetter-rpf zetter-rpf force-pushed the cache-submitted-count-part-2 branch from a503b60 to 7c74295 Compare May 18, 2026 09:23
@zetter-rpf zetter-rpf temporarily deployed to editor-api-p-cache-subm-k1rtmo May 18, 2026 09:24 Inactive
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR (part 2 of 2) leverages the submitted_projects_count counter cache column introduced in #804 to remove N+1 query patterns when listing lessons and classes. The custom submitted_count methods on Lesson and SchoolClass (which joined through projects, remixes, school_projects, and transitions) are replaced with reads of the cached column, and controller includes are simplified accordingly.

Changes:

  • Replaces Lesson#submitted_count usage with the cached submitted_projects_count column, and computes SchoolClass#submitted_projects_count as the sum over its (preloaded) lessons.
  • Simplifies eager-loading in LessonsController#index and SchoolClassesController#accessible_school_classes (no more loading the deep project → remixes → school_project → transitions chain for teacher views).
  • Updates jbuilder views and specs to use the new cached value (specs now set submitted_projects_count directly via the factory).

Reviewed changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file
File Description
app/models/lesson.rb Removes the old #submitted_count method that counted submitted remixes in Ruby.
app/models/school_class.rb Replaces the SQL-join-based #submitted_count with #submitted_projects_count summing the cached column over lessons.
app/controllers/api/lessons_controller.rb Drops deep includes(project: :remixes) for the teacher branch; keeps it only for the student branch that still needs remixes.
app/controllers/api/school_classes_controller.rb Replaces the deep includes chain with a simple includes(:lessons) so the sum can use preloaded lessons.
app/views/api/lessons/teacher_index.json.jbuilder Reads submitted_projects_count instead of submitted_count.
app/views/api/school_classes/teacher_index.json.jbuilder Reads submitted_projects_count instead of submitted_count.
spec/models/lesson_spec.rb Removes the now-obsolete #submitted_count examples (covered by counter-cache specs).
spec/models/school_class_spec.rb Rewrites the spec to set submitted_projects_count on lessons via the factory and asserts the summed value.
spec/features/lesson/listing_lessons_spec.rb Simplifies the listing spec by setting submitted_projects_count directly on the lesson.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@zetter-rpf zetter-rpf merged commit 294d991 into main May 18, 2026
9 checks passed
@zetter-rpf zetter-rpf deleted the cache-submitted-count-part-2 branch May 18, 2026 09:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants